ENG-9917 fix: run plugin post_compile hooks at most once per app instance#6733
ENG-9917 fix: run plugin post_compile hooks at most once per app instance#6733FarhanAliRaza wants to merge 3 commits into
Conversation
App.__call__ runs whenever the ASGI app is constructed, and it re-ran every plugin's post_compile each time. post_compile hooks mutate the app (add_middleware, mounting routes), so a second construction on the same app instance (a test harness rebuilding the ASGI app) re-applied those mutations — e.g. a middleware installed twice gates every event twice. Latch the hook loop per app instance, mirroring the once-per-process guarantees elsewhere in the compile path. The latch is set after the loop completes so a failed hook is retried on the next construction rather than silently skipped. The guarantee is documented on Plugin.post_compile so plugins don't need their own idempotency guards. Fixes ENG-9917.
Greptile SummaryThis PR changes how plugin post-compile hooks run during ASGI app construction. The main changes are:
Confidence Score: 4/5This is close, but the plugin cache path should be fixed before merging.
reflex/app.py Important Files Changed
Reviews (2): Last reviewed commit: "fix: assemble the ASGI app at most once ..." | Re-trigger Greptile |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 337341739e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not self._plugins_post_compiled: | ||
| for plugin in config.plugins: | ||
| plugin.post_compile(app=self) | ||
| self._plugins_post_compiled = True |
There was a problem hiding this comment.
Track completed post_compile hooks individually
Because the latch is only set after the whole plugin loop succeeds, a retry after a later plugin raises will start from the first plugin again. If an earlier plugin already mutated the app (for example by adding middleware or routes) and a subsequent plugin fails once, the next app() call duplicates the earlier mutation, so the new at-most-once guarantee still fails in the explicit retry-after-failure path.
Useful? React with 👍 / 👎.
Gating only the plugin post_compile loop left the same repeat-mutation bug elsewhere in App.__call__: the compiled-frontend mount was appended to the persistent self._api on every construction, and a Starlette api_transformer was re-mounted with an extra CORS layer each call. A single boolean latch also re-ran already-succeeded hooks when a later plugin's hook raised, reinstalling their middleware on retry. Extract the assembly steps (plugin hooks, frontend mount, transformer wiring, top-level app) into _assemble_asgi_app, cache its result per instance, and track completed hooks per plugin so a failed hook is retried without re-running the ones that already mutated the app. _compile intentionally still runs on every call: forked prod workers re-enter __call__ to re-evaluate stateful pages backend-side.
| # Assembly runs at most once per app instance; _compile stays per-call. | ||
| if self._cached_asgi_app is None: |
There was a problem hiding this comment.
Cached plugins stay skipped After the first successful
app() call, this gate returns the cached ASGI app and never enters _assemble_asgi_app() again. If get_config().plugins is changed on the same App instance between ASGI constructions, the new plugin is never inspected, so its post_compile hook cannot add its middleware or routes. The per-plugin set only helps when assembly runs again; this cache blocks that path entirely.
Fixes ENG-9917 at the framework level.
Problem
App.__call__runs every plugin'spost_compile(app=self)each time the ASGI app is constructed.post_compilehooks mutate the app —add_middleware, mounting routes — so constructing the ASGI app twice on the same app instance (a test harness rebuilding it, repeatedapp()calls) re-applies those mutations. The concrete instance that surfaced this: enterpriseAuthPlugininstalling a secondAuthMiddleware, gating every event twice. But the bug class belongs to the lifecycle, not to any one plugin — every app-mutatingpost_compilehook is affected.Fix
Latch the hook loop with a per-app-instance flag (
_plugins_post_compiled), sopost_compileruns at most once per app instance. The flag is set after the loop completes, so a hook that raises (e.g. a plugin rejecting a misconfigured app) is retried on the next construction instead of being silently skipped. The at-most-once guarantee is documented onPlugin.post_compileso plugin authors don't hand-roll idempotency guards.Backend-only processes (
REFLEX_SKIP_COMPILE) are unaffected: they construct a freshAppper process, and the flag starts false. Dev hot reload restarts the worker process (freshApp), so hooks still re-run there as before.Tests
test_call_app_runs_plugin_post_compile_once— regression test, failed before the fix (hook ran twice).test_call_app_retries_post_compile_after_failure— pins the not-latched-on-failure semantics.Full unit suite passes (6619, coverage 77.97%), ruff + pyright clean.